Refactor JPLSpec/CDMS to bring common code up a level #3456
Conversation
fix bad file add fix ****s
…; fix CDMS quantum numbers parsing 1) Adding support for CDMS queries with lines of all species 2) Fixing the CMDS lines list parsing Support CDMS all species option; fix format for CDMS linelist reading; fix CDMS quantum numbers parsing Adding test for a new functionality when all species are requested from CDMS fix parse_letternumber test and rearrange and refactor new test expand test coverage and resolve some problems discovered as a result propagate column change down trivial formatting fix my refactor; it was incorrect oops, fix to last one (yes, this needs to be squashed; pushing fast to skip tests... and spam my inbox...) add ch3cn test and shift QNFMT by one cleanup molwt/tag parsing shift tag back one spot. Fix tests to accommodate more complete "tag" name add the b1 = -21 test whitespace fix ch3cn test; it had decayed into ch3ccd which has different QNs add changelog improve error message for bad molecule parsing fix the next part of the test address review comments
|
My hacking has hit an awkward position. I wanted to implement a fallback-to-getmolecule approach, but this made me realize that the async-to-sync method prevents returning non-response results and prevents passing arguments directly to _parse_response. Both of these make the workaround a lot more awkward. There are a couple other approaches I could take, but I really want this fallback because of the unreliability of JPLspec and the hinted-at fragility of other services. |
|
What about ditching async to sync? I have my doubts it ever worked in a real async way |
|
yeah, I can get onboard with that. It's a big refactor, but not that hard. That said, I'd need to feel quite inspired / have little on my plate to want to take that on myself. I think it is not the tallest nail right now |
|
(also most of the problems I'm experiencing here are because the SPCAT format is extraordinarily arcane and has dozens of undocumented edge cases....) |
|
Of, I don't mean to refactor the machinery, but feel free to cut the usage of it. We don't use it in the vo backed modules anyway, so it's more like a slow cleanup rather than one big push. |
|
right, good point. OK, I'll think about that - once I get out of qn-parsing hell |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3456 +/- ##
==========================================
+ Coverage 70.72% 70.86% +0.13%
==========================================
Files 232 235 +3
Lines 20041 20209 +168
==========================================
+ Hits 14174 14321 +147
- Misses 5867 5888 +21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…l solution was wrong. Added inter-doc links
…e fallback logic is kinda complicated. But running the tests revealed an error in the code, so it's helpful already
|
I had an LLM write some of the tests to increase coverage. I reviewed those tests, and they look right, but I'm not certain. I can say that they revealed genuine bugs that I had to fix, so that was good. |
3 participants
WIP. I'm not as sure as I was at the start of this project that there is enough in common, but maybe.
This relies on other open PRs, so it's staying Draft for now